eh, why the fuck does it not work on travis?

sh: 1: scripts/build-module.js: not found

it’s right there! pls help!

Any way we can avoid duplicating Babel options? I'm pretty sure we'll forget to update them at some point unless they're all in .babelrc.

OK this works and is relatively elegant. it is all in .babelrc right now!

my only gripe: sadly either the tests fail or the module syntax gets modified in the bundle unless i duplicate everything inside of .babelrc.

{
  "env": {
    "development": {
      "presets": ["es2015-loose", "stage-0", "react"],
      "plugins": ["transform-decorators-legacy"]
    },
    "buildmodule": {
      "presets": ["es2015-loose-rollup", "stage-0", "react"],
      "plugins": ["transform-decorators-legacy"]
    }
  }
}

⇒ works!

{
  "env": {
    "development": {
      "presets": ["es2015-loose", "stage-0", "react"]
    },
    "buildmodule": {
      "presets": ["es2015-loose-rollup", "stage-0", "react"]
    }
  },
  "plugins": ["transform-decorators-legacy"]
}

⇒ SyntaxError: Decorators are not supported yet in 6.x pending proposal update.

{
  "env": {
    "development": {
      "presets": ["es2015-loose"]
    },
    "buildmodule": {
      "presets": ["es2015-loose-rollup"]
    }
  },
  "presets": ["stage-0", "react"],
  "plugins": ["transform-decorators-legacy"]
}

⇒ tests fail

{
  "env": {
    "buildmodule": {
      "presets": ["es2015-loose-rollup"]
    }
  },
  "presets": ["es2015-loose", "stage-0", "react"],
  "plugins": ["transform-decorators-legacy"]
}

⇒ module syntax gets compiled

i filed this babel bug for my troubles.

but i think this one is ripe for merging once you’re OK with dropping IE8 support. but it actually hasn’t been supported anymore for a week, so why wait 😉

A few comments:

• mod/ is not an ideal name; a number of packages use modules/ for their actual source (e.g. all the @rackt/routing packages). I think es6/ or es2015/ would be better.
• You can't just switch to ES2015 modules with Babel 6 unless you add https://www.npmjs.com/package/babel-plugin-add-module-exports; otherwise you break CJS users.
• It's really odd to me that the ES2015 build uses the external-helpers plugin but the CJS build doesn't; IMO the ES2015 build should be identical to the CJS build, save only for the lack of module transpliation.

mod/ is not an ideal name; a number of packages use modules/ for their actual source (e.g. all the @rackt/routing packages). I think es6/ or es2015/ would be better.

👍 on es6

You can't just switch to ES2015 modules with Babel 6 unless you add https://www.npmjs.com/package/babel-plugin-add-module-exports; otherwise you break CJS users.

This is not entirely accurate because we don't have a default export. The only reason we switched from ES6 modules to CommonJS a few releases ago is to work around Babel outputting .default rather than ['default'] which breaks in IE8. However we can fix this problem by putting https://github.com/sorrycc/es3ify-loader in our Webpack config.

It's really odd to me that the ES2015 build uses the external-helpers plugin but the CJS build doesn't; IMO the ES2015 build should be identical to the CJS build, save only for the lack of module transpliation.

I think I agree with this, curious to hear the reasons.

Ah – I got confused; looks like there wouldn't be any point in doing e.g. rackt-redux/lib/<whatever> imports, given that in practice people are going to want both Provider and connect anyway.

But if that's the case, what does adding a proper ES2015 module build add? If you always want everything in the library, the tree-shaking is going to be a no-op.

I set this up for React Router and for history because in those cases tree-shaking would actually accomplish something, since often users wouldn't want every module in those packages.

But if that's the case, what does adding a proper ES2015 module build add? If you always want everything in the library, the tree-shaking is going to be a no-op.

I don't know enough about Rollup to answer this. I thought it doesn't work with CommonJS builds at all, does it?

That doesn't sound right to me. React doesn't offer a ES2015 module build, does it?

Hm, good point. So then dropping jsnext:main should help here?

no, babel-created code is pretty hard to statically analyze. much if (package.default != null) ... and __esmodule stuff going on.

and i prefer modules because “es6” is confusing: that directory won’t contain ES6 code, just the module syntax, right?

Having jsnext:main pointing at untranspiled code will actively break Rollup users (and anybody else looking at that field).

I'll have to defer to @flying-sheep as to whether there are any benefits to having an ES2015 module build aside from the possibility of tree-shaking. I think tree-shaking matters a lot for allowing e.g.

import { createBrowserHistory, useBasename, useQueries } from 'history';

over the much uglier

import createBrowserHistory from 'history/lib/createBrowserHistory';
import useBasename from 'history/lib/useBasename';
import useQueries from 'history/lib/useQueries';

for minimizing bundle size, but that doesn't seem relevant here if everybody needs both <Provider> and connect().

Separately, I'd say mod/ might make sense in a vacuum, but unfortunately modules/ already is used for non-transpiled source on a number of projects – es6 at least conveys that you're using ES6 modules rather than CJS ones.

no, babel-created code is pretty hard to statically analyze.

Why would it need to be analyzed? We can't benefit from tree shaking in this project.

unfortunately modules/ already is used for non-transpiled source on a number of projects

ah, ok. others simply ship src/.

Why would it need to be analyzed? We can't benefit from tree shaking in this project.

irrelevant. the important part is that the code babel creates is too complicated for rollup-plugin-commonjs to understand. (rollup-plugin-commonjs basically converts commonjs require() calls to ES2015 module syntax)

so if we want to be static-analysis-friendly, we need to ship simple commonjs (const foo = require('foo')) or ES2015 module syntax.

i prefer the latter, because the former leads to guesswork if const foo = require('foo') means import foo from 'foo' or import * as foo from 'foo'.

as is, rollup users simply can’t use any npm thing that uses babel-compiled module syntax.

Sure, but there are plenty of packages that do use babel-compiled module syntax, and nevertheless they are used by rollup users.

What do you get from static analysis other than tree-shaking?

Sure, but there are plenty of packages that do use babel-compiled module syntax, and nevertheless they are used by rollup users.

well, it works if you’re lucky. else you’ll end up with this (discussion)

btw i’m really discouraged right now. rollup seems really promising, but this discussion dragged on far too long already to have any hope in rollup becoming practical.

i really don’t want to have endless discussion with every package maintainer about this, but that will be inevitable i guess.

Well, no, for some packages having an ES2015 module build is obviously beneficial for both Rollup and webpack 2 users. I've personally added ES2015 module builds to both React Router and history.

That said, there is some non-zero cost to the module maintainer to maintain an additional build. From my perspective, if there are meaningful benefits available through tree-shaking, as there are for my other packages like React Router, history, and React-Bootstrap, then I'm happy to add the ES2015 module build.

For something where there's really only one export, like e.g. react-router-relay, or possibly react-redux? I think there needs to be a concrete benefit to adding another build target.

If nothing else I hope this discussion will demonstrate good general guidelines for other projects in considering whether to add an ES2015 module build.

If nothing else I hope this discussion will demonstrate good general guidelines for other projects in considering whether to add an ES2015 module build.

you’re right, that occurred to me as well. we’re treading unexplored ground right here, so it’s probably not bad to have some discussion now and a “best practices” thing in the end.

I think there needs to be a concrete benefit to adding another build target.

IMHO there is. it’s the difference between a default export and grab-bag-of-stuff exports. import * as name vs import name.

rollup handles those as strict as the real deal in the future. you won’t be able to do import React, { Component } from 'react' if there’s not both individual and default exports. babel however allows that.

currently i have to go to node_modules or even github and look into the ES6 source code to see which export style is intended by the authors.

Commonjs requires some extra code in bundle. Even without tree shaking result will be smaller.

@flying-sheep

I don't think that's correct. Looking at the implementation of _interopRequireDefault, you wouldn't be able to do e.g.

import ReactRedux from 'react-redux';

At this point in time, I also don't think the extra bundle size purely from using CJS modules rather than ES6 ones (when tree shaking is not part of the picture) is really all that meaningful.

forget rollup. forget bundling. static analysis of dependencies is important for tooling.

think dependency injection. think asynchronous loading. or simply believe me that all the other languages with dedicated import statements have a reason for that

_interopRequireDefault is a function call, not static syntax. therefore we need ES2105 module syntax in order to advance the JS ecosystem.

We do, yes, but there's no accounting for timing. Eventually Node is going to have actual support for ES2015 modules, and webpack 2 will have support for the same – at which point it'd be possible to distribute just an ES2015 module build, and not have both that and a CJS build.

don’t forget people who’ll distribute node 0.12 stuff for backwards compatibility

so back to topic: when renaming the module directory to es2015 or es6, is everyone confident that this is a good way to go?

if not, please speak up and voice your concerns, and ideally propose alternatives.

If you have a hard dependency on a specific Node version for your React code, you're doing something very odd/wrong.

I just feel like, in the absence of a compelling bundle size benefit from tree shaking, that it's not really worthwhile to maintain two transpiled builds – ideally at some point in the future it will be possible to just swap out the CJS module build for an ES2015 module build.

It's not "advancing the JS ecosystem" to just set up some awkward shims that most people can't take advantage of – Rollup is not an option in general for packaging a full-fledged application, and webpack 2 isn't ready yet.

Rollup is not an option in general for packaging a full-fledged application

because not enough npm modules support jsnext:main, right?

No, because real applications tend to need things like CSS and other assets.

how is this relevant?

What would one use webpack, Browserify, or Rollup for, if not to generate client-side bundles?

sure, but either you use JSS and so on or bundle assets independently

Either way, most users are currently simply not going to be able to take advantage of an ES2015 module build. Maybe when webpack 2 leaves beta, assuming it doesn't hit the same teething issues as Babel 6, and/or when Node cuts a stable release that supports this module syntax.

I'd happily encourage and even push people toward using proper ES2015 modules over the transpiled CJS stuff once it's something that has the potential for broad adoption, but we're not at that point yet, and "it feels cleaner" is not IMO enough of a benefit to make the build pipeline more complicated on all the libraries.

We should save it for the ones where tree shaking gives a concrete bundle size reduction.

I'd also prefer we use https://github.com/gajus/babel-preset-es2015-webpack (perhaps a loose version?) over the rollup preset, since the webpack one doesn't introduce the extra babel external transform, and more of our users will likely be using webpack than rollup.

This is recommended by @sokra too: https://gist.github.com/sokra/27b24881210b56bbaff7#babel-and-webpack

However, we aren't using presets on the project, so it's just a matter of removing this line.

the rollup preset’s transform externalizes the babel-helpers, that’s all, but OK.

we also need this babel issue fixed if we want to parameterize the inclusion of the module transform

I don’t really understand what the deal with Babel external helpers is, so I can’t move forward here.
Please help me understand the issue.
For now, I released React Redux that removes broken jsnext:main because it didn’t help anybody.

For now, I released React Redux that removes broken jsnext:main because it didn’t help anybody.

thank you!

I don’t really understand what the deal with Babel external helpers is, so I can’t move forward here.

Please help me understand the issue.

here and here is the explanation:

the plugin makes the individual compiled files rely on a “babelHelpers” variable. rollup-plugin-babel detects if this is the case (but also handles other ways to include babel helpers, warning if necessary).

it’s useful to not include them more than once.

Can we pull in the tooling from the main Redux library, then figure out across-the-board what to do with external helpers?

I don't think it'd be good to have an ES2015 module build that only works with Rollup and not with webpack 2.

FYI, babel 6.6.0 has fixed the exports default problem in IE8.
https://github.com/babel/babel/blob/master/CHANGELOG.md#exportsdefault-fix

I’m closing as this is rather outdated. We can revisit later if there is enough desire to do so.